-
Notifications
You must be signed in to change notification settings - Fork 132
[Dynamic Dashboard] Fix issue of not updating the orders count in some scenarios #11530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is done by updating the logic to fetch the order status options instead of the `/wc-analytics/reports/orders/totals`
We already do a fetch when the app starts in the SiteObserver, so here we should just use the cached value initially, and then observe changes
Generated by 🚫 Danger |
// emit updated value | ||
fetchOrdersCount(site)?.let { emit(it) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we fetch the order status options on app launch in SiteObserver
, making another fetch here is redundant, so I removed it.
Now we will start with the cached value, then observe changes and fetch when needed.
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #11530 +/- ##
============================================
- Coverage 40.47% 40.46% -0.01%
+ Complexity 5181 5180 -1
============================================
Files 1075 1075
Lines 62873 62872 -1
Branches 8612 8606 -6
============================================
- Hits 25447 25444 -3
- Misses 35138 35141 +3
+ Partials 2288 2287 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started out with a store with no orders (there are some trashed orders) and the cards are still being shown, even after force-refreshing the orders and the dashboard. 🤔
Screen_recording_20240517_095857.webm
@0nko thanks for the test, can you invite me to your store to check this? This doesn't happen on my store, |
We skip the initial orders count, with this we don't make a redundant API call to fetch the order status options, something that SiteObserver already handles.
The function checks only the local DB, and given how pagination works, we don't remove trashed orders from the API, so when a site has trashed orders, we keep thinking that they have valid orders, and we keep showing the stats cards.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well after the last change. Nice fix!
Description
In some cases, and given the fact that order status options are fetched only on app launch and when switching sites, it could happen that the app shows the performance card for stores with no orders or vice versa.
This PR fixes this by making two changes:
ObserveProcessingOrdersCount
to always use the order status API for infering the number of processing orders, instead of using the/wc-analytics/reports/totals
endpoint, this update will allow us to refresh the cache whenever any changes happen.ObserveSiteOrdersState
to observe the changes of order status options instead of retrieving a single snapshot.With these two changes, the dashboard will always observe the updated state.
Testing instructions
undo
action that we use before trashing the order).Images/gif
Screen_recording_20240516_115549.mp4
RELEASE-NOTES.txt
if necessary. Use the "[Internal]" label for non-user-facing changes.